Skip to content

Conversation

@JLLeitschuh
Copy link
Member

Rewrite of @MeRPG's PR.

Closes #529

@JLLeitschuh JLLeitschuh force-pushed the feat/MeRPG_operations branch from d7a7b00 to c2b0a9f Compare June 3, 2016 18:56
@codecov-io
Copy link

Current coverage is 65.30%

Merging #600 into master will increase coverage by 0.08%

  1. File ...ables/NTManager.java (not in diff) was modified. more
    • Misses +3
    • Partials 0
    • Hits -3
@@             master       #600   diff @@
==========================================
  Files           175        177     +2   
  Lines          5415       5479    +64   
  Methods           0          0          
  Messages          0          0          
  Branches        519        524     +5   
==========================================
+ Hits           3529       3578    +49   
- Misses         1886       1901    +15   
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by 2c2c52d...d7a7b00

@JLLeitschuh JLLeitschuh force-pushed the feat/MeRPG_operations branch from c2b0a9f to df667d5 Compare June 3, 2016 19:15
@SamCarlberg
Copy link
Member

Why do we need an operation to count contours? Why not just add a size field to the existing publishing operations?

@AustinShalit
Copy link
Member

Number Threshold and Count Contours would be good examples of python operations but should not be added as GRIP operations.

I am ok with the crop operation.

@AustinShalit AustinShalit added this to the v2.0.0 milestone Aug 3, 2016
SocketHints.Inputs.createMatSocketHint("src", false),
SocketHints.Inputs.createPointSocketHint("p1", false),
SocketHints.Inputs.createPointSocketHint("p2", false),
SocketHints.Inputs.createMatSocketHint("dst", true),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this make the dst an Input?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. But the Crop operation should be in its own class file like every other custom operation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait... SocketHints.Inputs.createMatSocketHint() can be used to create a socket hint used for an output? Isn't that a bit misleading?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. SocketHints needs an overhaul (and Javadocs).

Copy link
Member Author

@JLLeitschuh JLLeitschuh Aug 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How so? What are you thinking about this? I agree with needing an overhaul but I want to know what your thoughts are. This overhaul needs to be done before we expose python bindings.
Open an issue and we can discuss that there.

@SamCarlberg
Copy link
Member

SamCarlberg commented Aug 3, 2016

👎 Custom operations should be in their own class files.

@AustinShalit AustinShalit modified the milestones: Future, v1.5.0 Aug 27, 2016
@AustinShalit
Copy link
Member

I am going to close this PR because it does not look like this is going to happen in the near future. Please reopen if you have new information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants